Send plugin names to Core #1157
Conversation
…untime client identity requirement
| activity_task_poller_behavior: PollerBehavior | ||
| nexus_task_poller_behavior: PollerBehavior | ||
| plugins: Sequence[Plugin] | ||
| skip_client_worker_set_check: bool |
There was a problem hiding this comment.
I don't love the addition of this to the very public worker config. This was due to validation issues in the tests, correct? How much work do we think it would be to actually just fix those issues? Is there any place where it is really meaningful?
There was a problem hiding this comment.
we want to set skip_client_worker_set_check to false for tests. I didn't check how python does things, but in Core, since tests run in parallel and use the same namespace, we were hitting errors from that. No reason to have this check for tests, unless we're explicitly trying to test for it, which we already do in Core.
I can try to remove from here
There was a problem hiding this comment.
Makes sense in Core, but exposing it in the public API wouldn't be fun. Let's see if we can find another way around it.
There was a problem hiding this comment.
Struggling to think of a way around this, our tests pass in WorkerConfig as a typed dict, and Worker::init also uses the WorkerConfig typed dict.
I feel like it should be fine to expose this publically, for users writing their own tests, if they run their tests in parallel on the same namespace/task queue, they would also want to disable this check
| ) | ||
| .nexus_task_poller_behavior(conf.nexus_task_poller_behavior) | ||
| .plugins( | ||
| conf.plugins |
There was a problem hiding this comment.
So, something that will need some discussion here. This PR reports worker plugins. We should discuss whether that is really what we intend. Plugins which exist only in the client and not the worker will be completely invisible. That could potentially be changed here at least for plugins in clients used by workers, though not generally for any client. I think that was something we didn't really think through when we decided to go with heartbeat as a carrier for this information, but maybe we conclude that is fine.
Typescript will be a bit more complicated as well with its additional plugin types.
There was a problem hiding this comment.
Discussed offline, we will for now have both worker and client plugins and dedup names
3 participants
What was changed
Send plugin names over to core for worker heartbeating.
Updated Core to latest main, 9e9a461.
Updated test to validate replacing clients with a client from a different runtime is invalid.
Why?
Worker heartbeating
Checklist
Closes
How was this tested:
Added tests for plugin name propogation, and runtime options configuration
Note
Forward worker plugin names to Core and add runtime options (incl. worker heartbeat interval), alongside dependency bumps and test updates.
init_runtime(telemetry)withinit_runtime(options); introduceRuntimeOptions(telemetry +worker_heartbeat_interval_millis).Runtimenow acceptsworker_heartbeat_intervaland buildsRuntimeOptionsfor the bridge.RuntimeOptionsBuilderand propagate heartbeat interval.pluginsandskip_client_worker_set_checktoWorkerConfig(Python and Rust bridge); send plugin names to Core (PluginInfo).Client.pluginsproperty.Worker.replace_clientnow returns error on failure; Python wrapper validates same-runtime client when replacing.TemporalServiceClientimport path; minor type updates.poe lintruns after build in both main and latest-deps jobs.opentelemetry,tonic(to 0.14),prost(to 0.14), introducetonic-prost*,dyn-clone, others; update sdk-core submodule.Written by Cursor Bugbot for commit ebc6c0d. This will update automatically on new commits. Configure here.